feat(sdk): replace onStart lifecycle hook with onStartAttempt #2515
Conversation
🦋 Changeset detectedLatest commit: 23930b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ericallam
force-pushed
the
ea-branch-88
branch
from
bbf70ee to
6170b3d
Compare
WalkthroughThis pull request introduces a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-08-18T10:07:17.368ZApplied to files:
📚 Learning: 2025-08-18T10:07:17.368ZApplied to files:
📚 Learning: 2025-08-18T10:07:17.368ZApplied to files:
📚 Learning: 2025-08-18T10:07:17.368ZApplied to files:
🧬 Code graph analysis (1)packages/core/src/v3/types/tasks.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
.changeset/tiny-carrots-rest.md (4)
2-3: Include all affected packages in the changesetCore types and APIs were added; ensure
@trigger.dev/core(and any other published packages touched) receive a bump in this PR’s changesets, not just@trigger.dev/sdk.
20-27: Clarify file location for global hooksDocs should state that global lifecycle hooks belong in
trigger.config.tsper established guidance.Apply this doc tweak:
-// Default a global lifecycle hook using tasks +// Default a global lifecycle hook using tasks (in trigger.config.ts) tasks.onStartAttempt(({ ctx, payload, task }) => {
31-40: Code-fence filename syntax may not render
```ts /trigger/on-start-attempt.tsmight not be recognized by your MDX tooling. Prefertitle=or a preceding caption.
5-5: Specify logging/telemetry behavior for swallowed errorsIf onSuccess/onComplete/onFailure errors no longer fail runs, note that they’re still captured in logs/metrics.
packages/core/src/v3/lifecycle-hooks-api.ts (1)
7-39: Re-export full StartAttempt types for consistencyYou currently export only
AnyOnStartAttemptHookFunction. Re-exportOnStartAttemptHookFunctionandTaskStartAttemptHookParamslike other hooks to aid typing.export type { OnInitHookFunction, AnyOnInitHookFunction, RegisteredHookFunction, TaskInitHookParams, TaskStartHookParams, OnStartHookFunction, AnyOnStartHookFunction, + // StartAttempt + OnStartAttemptHookFunction, + TaskStartAttemptHookParams, TaskFailureHookParams, AnyOnFailureHookFunction, TaskSuccessHookParams, AnyOnSuccessHookFunction, TaskCompleteHookParams, AnyOnCompleteHookFunction, TaskWaitHookParams, AnyOnWaitHookFunction, TaskResumeHookParams, AnyOnResumeHookFunction, TaskCatchErrorHookParams, AnyOnCatchErrorHookFunction, TaskCompleteResult, TaskMiddlewareHookParams, AnyOnMiddlewareHookFunction, OnMiddlewareHookFunction, OnCleanupHookFunction, AnyOnCleanupHookFunction, TaskCleanupHookParams, TaskWait, TaskCancelHookParams, OnCancelHookFunction, AnyOnCancelHookFunction, AnyOnStartAttemptHookFunction, } from "./lifecycleHooks/types.js";packages/core/src/v3/types/tasks.ts (1)
338-349: Consider clarifying the deprecation timeline foronStart.While the deprecation notice directs users to
onStartAttempt, consider adding information about whenonStartwill be removed to help users plan their migration.Consider enhancing the deprecation notice:
/** * onStart is called the first time a task is executed in a run (not before every retry) * - * @deprecated Use onStartAttempt instead + * @deprecated Use onStartAttempt instead. This will be removed in v5.0.0 */docs/tasks/overview.mdx (1)
359-362: Consider documenting where these suppressed errors are visible in the dashboard.The documentation mentions that errors in onSuccess, onComplete, and onFailure are ignored but visible in the dashboard. Consider adding more specific information about where users can find these errors (e.g., in the run details, logs panel, etc.).
Would you like me to suggest more specific documentation about where these errors appear in the dashboard UI?
Also applies to: 390-393, 421-424
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
references/hello-world/src/trigger/example.tsis excluded by!references/**
📒 Files selected for processing (14)
.changeset/tiny-carrots-rest.md(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(1 hunks)docs/tasks/overview.mdx(9 hunks)packages/cli-v3/package.json(1 hunks)packages/core/src/v3/lifecycle-hooks-api.ts(1 hunks)packages/core/src/v3/lifecycleHooks/index.ts(2 hunks)packages/core/src/v3/lifecycleHooks/manager.ts(4 hunks)packages/core/src/v3/lifecycleHooks/types.ts(2 hunks)packages/core/src/v3/types/tasks.ts(4 hunks)packages/core/src/v3/workers/taskExecutor.ts(8 hunks)packages/core/test/taskExecutor.test.ts(2 hunks)packages/trigger-sdk/src/v3/hooks.ts(2 hunks)packages/trigger-sdk/src/v3/shared.ts(2 hunks)packages/trigger-sdk/src/v3/tasks.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/core/src/v3/lifecycle-hooks-api.tsapps/webapp/app/components/runs/v3/RunIcon.tsxpackages/trigger-sdk/src/v3/shared.tspackages/trigger-sdk/src/v3/tasks.tspackages/core/src/v3/lifecycleHooks/types.tspackages/core/src/v3/lifecycleHooks/index.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/types/tasks.tspackages/trigger-sdk/src/v3/hooks.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/src/v3/lifecycleHooks/manager.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/lifecycle-hooks-api.tsapps/webapp/app/components/runs/v3/RunIcon.tsxpackages/core/src/v3/lifecycleHooks/types.tspackages/core/src/v3/lifecycleHooks/index.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/types/tasks.tspackages/core/src/v3/workers/taskExecutor.tspackages/core/src/v3/lifecycleHooks/manager.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/components/runs/v3/RunIcon.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
packages/core/test/taskExecutor.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests must use Vitest
Tests should avoid mocks or stubs and use helpers from @internal/testcontainers when Redis or Postgres are needed
Test files live beside the files under test and should use descriptive describe and it blocks
Files:
packages/core/test/taskExecutor.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to trigger.config.ts : Configure global task lifecycle hooks (onStart/onSuccess/onFailure) only within trigger.config.ts if needed, not within arbitrary files
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to trigger.config.ts : Configure global task lifecycle hooks (onStart/onSuccess/onFailure) only within trigger.config.ts if needed, not within arbitrary files
Applied to files:
.changeset/tiny-carrots-rest.mdpackages/trigger-sdk/src/v3/shared.tspackages/trigger-sdk/src/v3/tasks.tspackages/core/test/taskExecutor.test.tspackages/core/src/v3/types/tasks.tsdocs/tasks/overview.mdxpackages/trigger-sdk/src/v3/hooks.tspackages/core/src/v3/workers/taskExecutor.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Define tasks using task({ id, run, ... }) with a unique id per project
Applied to files:
.changeset/tiny-carrots-rest.mdpackages/trigger-sdk/src/v3/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use triggerAndWait() only from within a task context (not from generic app code) and handle result.ok or use unwrap() with error handling
Applied to files:
.changeset/tiny-carrots-rest.mdpackages/trigger-sdk/src/v3/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Do not use client.defineJob or any deprecated v2 patterns (e.g., eventTrigger) when defining tasks
Applied to files:
.changeset/tiny-carrots-rest.mdpackages/trigger-sdk/src/v3/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
.changeset/tiny-carrots-rest.mdpackages/trigger-sdk/src/v3/shared.tspackages/trigger-sdk/src/v3/tasks.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use schedules.task(...) for scheduled (cron) tasks; do not implement schedules as plain task() with external cron logic
Applied to files:
packages/trigger-sdk/src/v3/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task multiple times in a loop from inside another task, use batchTrigger()/batchTriggerAndWait() instead of per-item trigger() calls
Applied to files:
packages/trigger-sdk/src/v3/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use schemaTask({ schema, run, ... }) to validate payloads when input validation is required
Applied to files:
packages/core/src/v3/types/tasks.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export every task (including subtasks) defined with task(), schedules.task(), or schemaTask()
Applied to files:
docs/tasks/overview.mdx
🧬 Code graph analysis (7)
packages/trigger-sdk/src/v3/shared.ts (2)
packages/core/src/v3/lifecycle-hooks-api.ts (2)
lifecycleHooks(5-5)AnyOnStartAttemptHookFunction(38-38)packages/core/src/v3/lifecycleHooks/types.ts (1)
AnyOnStartAttemptHookFunction(47-47)
packages/core/src/v3/lifecycleHooks/index.ts (2)
packages/core/src/v3/lifecycleHooks/types.ts (3)
RegisterHookFunctionParams(182-185)AnyOnStartAttemptHookFunction(47-47)RegisteredHookFunction(187-191)packages/core/src/v3/lifecycle-hooks-api.ts (2)
AnyOnStartAttemptHookFunction(38-38)RegisteredHookFunction(10-10)
packages/core/test/taskExecutor.test.ts (3)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
lifecycleHooks(5-5)packages/core/src/v3/workers/taskExecutor.ts (12)
payload(364-416)payload(418-454)payload(682-782)payload(784-849)payload(851-916)payload(918-928)payload(930-994)payload(996-1058)payload(1060-1068)payload(1070-1140)payload(1352-1417)result(1303-1350)packages/core/src/v3/types/tasks.ts (1)
RunFnParams(89-96)
packages/core/src/v3/types/tasks.ts (3)
packages/trigger-sdk/src/v3/shared.ts (1)
Context(122-122)packages/core/src/v3/lifecycleHooks/types.ts (2)
OnStartHookFunction(30-32)OnStartAttemptHookFunction(43-45)packages/core/src/v3/workers/taskExecutor.ts (11)
payload(364-416)payload(418-454)payload(682-782)payload(784-849)payload(851-916)payload(918-928)payload(930-994)payload(996-1058)payload(1060-1068)payload(1070-1140)payload(1352-1417)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycle-hooks-api.ts (3)
AnyOnStartHookFunction(14-14)AnyOnStartAttemptHookFunction(38-38)lifecycleHooks(5-5)packages/core/src/v3/lifecycleHooks/types.ts (2)
AnyOnStartHookFunction(34-34)AnyOnStartAttemptHookFunction(47-47)
packages/core/src/v3/workers/taskExecutor.ts (5)
packages/core/src/utils.ts (1)
tryCatch(5-18)packages/core/src/v3/schemas/common.ts (2)
TaskRunContext(407-417)TaskRunContext(419-419)packages/core/src/v3/lifecycle-hooks-api.ts (1)
lifecycleHooks(5-5)packages/core/src/v3/run-timeline-metrics-api.ts (1)
runTimelineMetrics(5-5)packages/core/src/v3/semanticInternalAttributes.ts (1)
SemanticInternalAttributes(1-64)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (3)
RegisteredHookFunction(187-191)AnyOnStartAttemptHookFunction(47-47)RegisterHookFunctionParams(182-185)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
packages/cli-v3/package.json (1)
158-158: Trailing newline only — OKNo functional change. Safe to merge.
packages/core/src/v3/lifecycleHooks/types.ts (2)
36-48: New StartAttempt hook types look goodShape matches intent (no
init), return type aligned with other hooks.
285-294: Manager surface extended — ensure Noop/impl parityConfirm
NoopLifecycleHooksManagerand the concrete manager implement these four new methods to avoid runtime undefined-method errors.packages/trigger-sdk/src/v3/tasks.ts (1)
92-95: API addition looks good; deprecation marker is clear
tasks.onStartAttemptis exposed next toonStartwith a deprecation note. Back-compat maintained.packages/trigger-sdk/src/v3/shared.ts (1)
91-95: Per-task StartAttempt hook registration wired correctly — verifiedType cast and registration follow existing patterns: core types declare onStartAttempt (packages/core/src/v3/types/tasks.ts), lifecycle registration exists (packages/core/src/v3/lifecycleHooks/index.ts, manager.ts), and taskExecutor invokes the hook (packages/core/src/v3/workers/taskExecutor.ts); tests exercise hook ordering.
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
100-107: Approve — event name confirmedEmitted span/icon is exactly "task-hook-onStartAttempt" (packages/core/src/v3/workers/taskExecutor.ts:1017, 1044) and RunIcon maps it (apps/webapp/app/components/runs/v3/RunIcon.tsx:100).
packages/core/test/taskExecutor.test.ts (2)
272-355: LGTM! The onStartAttempt test validates the new per-attempt lifecycle hook.The test correctly:
- Registers global and task-specific onStartAttempt hooks
- Verifies hooks are called in the expected order (global-1, global-2, task)
- Confirms each hook receives the correct payload
- Validates that the init data is properly accessible in the run function
1334-1374: LGTM! Error suppression for onSuccess hooks is correctly implemented.The test confirms that errors thrown in onSuccess hooks do not propagate and fail the run, which aligns with the documented behavior. The run completes successfully despite the hook error.
packages/core/src/v3/types/tasks.ts (2)
118-123: LGTM! StartAttemptFnParams type is consistent with existing patterns.The new type follows the same structure as
StartFnParams, providing context, optional init data, and the abort signal.
932-932: LGTM! TaskMetadataWithFunctions correctly includes the onStartAttempt function.The type signature matches the expected pattern with payload and StartAttemptFnParams.
packages/core/src/v3/workers/taskExecutor.ts (6)
186-186: LGTM! The onStartAttempt hook is correctly invoked after the first-attempt check.The placement ensures the hook runs on every attempt (including the first), which aligns with the documented behavior.
800-846: LGTM! Error suppression for onSuccess hooks is implemented correctly.The removal of error propagation ensures that hook failures don't fail the run, matching the documented behavior and test expectations.
867-913: LGTM! Error suppression for onFailure hooks is consistent.The implementation matches the onSuccess pattern, ensuring hook errors don't propagate.
1368-1414: LGTM! Error suppression for onComplete hooks follows the same pattern.Consistent implementation across all three hook types (onSuccess, onFailure, onComplete).
694-773: LGTM! The init hook merging logic is well-implemented.The implementation correctly:
- Collects results from multiple global init hooks
- Merges them with Object.assign
- Allows task-specific init hooks to override global results
- Preserves error propagation for init hooks (as they should fail the attempt)
996-1058: LGTM! The onStartAttempt implementation follows established patterns.The new hook implementation:
- Correctly retrieves both global and task-specific hooks
- Executes them in the right order
- Properly propagates errors (as start-attempt errors should fail the attempt)
- Uses appropriate telemetry attributes
packages/core/src/v3/lifecycleHooks/index.ts (1)
85-104: LGTM! StartAttempt hook registration methods follow the established pattern.The new methods correctly delegate to the lifecycle hooks manager and maintain consistency with existing hook registration patterns.
docs/tasks/overview.mdx (2)
250-294: LGTM! The onStartAttempt documentation is comprehensive and clear.The documentation:
- Clearly explains the new hook's purpose
- Shows both per-task and global usage examples
- Provides guidance on detecting the first attempt
- Includes important error handling information
538-571: LGTM! The deprecation notice for onStart is clear and helpful.The deprecated section:
- Clearly marks the feature as deprecated
- Provides migration guidance to onStartAttempt
- Retains the documentation for existing users
- Explains the key difference (once per run vs. per attempt)
packages/core/src/v3/lifecycleHooks/manager.ts (3)
17-18: LGTM: added import for AnyOnStartAttemptHookFunctionImport is correct and consistent with existing patterns.
27-35: LGTM: start-attempt hook maps addedGlobal and per‑task maps mirror existing start/init patterns. Single per‑task hook aligns with current design elsewhere.
142-171: LGTM: standard manager APIs for start-attempt hooksRegistration and retrieval methods follow established conventions (id generation, return shapes). No issues spotted.
…nFailure hooks to fail attempts & in some cases runs
ericallam
force-pushed
the
ea-branch-88
branch
from
494b659 to
6984903
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/trigger-sdk/src/v3/hooks.ts (1)
45-56: Overload types corrected foronStartAttempt— nice.The overloads now use
AnyOnStartAttemptHookFunction; resolves the prior mismatch.
🧹 Nitpick comments (4)
packages/core/src/v3/types/tasks.ts (1)
344-350: Fix first-attempt check in docs comment.Use
ctx.run.attempt.number, notctx.attempt.number.- * You can detect the first attempt by checking `ctx.attempt.number === 1`. + * You can detect the first attempt by checking `ctx.run.attempt.number === 1`.packages/trigger-sdk/src/v3/hooks.ts (1)
15-16: Re-exportAnyOnStartAttemptHookFunctionfor API parity.Other hook types are re-exported below; add this one too for consistency.
export type { AnyOnStartHookFunction, TaskStartHookParams, OnStartHookFunction, AnyOnFailureHookFunction, AnyOnSuccessHookFunction, AnyOnCompleteHookFunction, TaskCompleteResult, AnyOnWaitHookFunction, AnyOnResumeHookFunction, AnyOnCatchErrorHookFunction, AnyOnMiddlewareHookFunction, AnyOnCancelHookFunction, + AnyOnStartAttemptHookFunction, };docs/tasks/overview.mdx (2)
257-266: Rename code block path to match the feature.The section is about
onStartAttemptbut the snippet is labeled/trigger/on-start.ts.-```ts /trigger/on-start.ts +```ts /trigger/on-start-attempt.ts export const taskWithOnStartAttempt = task({ id: "task-with-on-start-attempt", onStartAttempt: async ({ payload, ctx }) => { //... },
380-388: Correct the earlier onComplete example (uses undefinedresult).The snippet above in this section references
resultbut destructures{ payload, output, ctx }. Adjust to use aresultparam or update the example; consider:onComplete: async ({ result, ctx }) => { if (result.ok) { console.log("Run succeeded", result.output); } else { console.log("Run failed", result.error); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/images/lifecycle-functions.pngis excluded by!**/*.pngreferences/hello-world/src/trigger/example.tsis excluded by!references/**
📒 Files selected for processing (14)
.changeset/tiny-carrots-rest.md(1 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(1 hunks)docs/tasks/overview.mdx(9 hunks)packages/cli-v3/package.json(1 hunks)packages/core/src/v3/lifecycle-hooks-api.ts(1 hunks)packages/core/src/v3/lifecycleHooks/index.ts(2 hunks)packages/core/src/v3/lifecycleHooks/manager.ts(4 hunks)packages/core/src/v3/lifecycleHooks/types.ts(2 hunks)packages/core/src/v3/types/tasks.ts(4 hunks)packages/core/src/v3/workers/taskExecutor.ts(8 hunks)packages/core/test/taskExecutor.test.ts(2 hunks)packages/trigger-sdk/src/v3/hooks.ts(2 hunks)packages/trigger-sdk/src/v3/shared.ts(2 hunks)packages/trigger-sdk/src/v3/tasks.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- .changeset/tiny-carrots-rest.md
- packages/core/src/v3/lifecycle-hooks-api.ts
- packages/cli-v3/package.json
- apps/webapp/app/components/runs/v3/RunIcon.tsx
- packages/core/src/v3/lifecycleHooks/types.ts
- packages/core/src/v3/lifecycleHooks/manager.ts
- packages/trigger-sdk/src/v3/tasks.ts
- packages/core/src/v3/lifecycleHooks/index.ts
- packages/trigger-sdk/src/v3/shared.ts
- packages/core/test/taskExecutor.test.ts
- packages/core/src/v3/workers/taskExecutor.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/trigger-sdk/src/v3/hooks.tspackages/core/src/v3/types/tasks.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/types/tasks.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to trigger.config.ts : Configure global task lifecycle hooks (onStart/onSuccess/onFailure) only within trigger.config.ts if needed, not within arbitrary files
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to trigger.config.ts : Configure global task lifecycle hooks (onStart/onSuccess/onFailure) only within trigger.config.ts if needed, not within arbitrary files
Applied to files:
packages/trigger-sdk/src/v3/hooks.tspackages/core/src/v3/types/tasks.tsdocs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use schemaTask({ schema, run, ... }) to validate payloads when input validation is required
Applied to files:
packages/core/src/v3/types/tasks.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Define tasks using task({ id, run, ... }) with a unique id per project
Applied to files:
docs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use schedules.task(...) for scheduled (cron) tasks; do not implement schedules as plain task() with external cron logic
Applied to files:
docs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use triggerAndWait() only from within a task context (not from generic app code) and handle result.ok or use unwrap() with error handling
Applied to files:
docs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task multiple times in a loop from inside another task, use batchTrigger()/batchTriggerAndWait() instead of per-item trigger() calls
Applied to files:
docs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export every task (including subtasks) defined with task(), schedules.task(), or schemaTask()
Applied to files:
docs/tasks/overview.mdx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Do not use client.defineJob or any deprecated v2 patterns (e.g., eventTrigger) when defining tasks
Applied to files:
docs/tasks/overview.mdx
🧬 Code graph analysis (2)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycle-hooks-api.ts (2)
AnyOnStartAttemptHookFunction(38-38)lifecycleHooks(5-5)packages/core/src/v3/lifecycleHooks/types.ts (1)
AnyOnStartAttemptHookFunction(47-47)
packages/core/src/v3/types/tasks.ts (3)
packages/trigger-sdk/src/v3/shared.ts (1)
Context(122-122)packages/core/src/v3/lifecycleHooks/types.ts (2)
OnStartHookFunction(30-32)OnStartAttemptHookFunction(43-45)packages/core/src/v3/workers/taskExecutor.ts (11)
payload(364-416)payload(418-454)payload(682-782)payload(784-849)payload(851-916)payload(918-928)payload(930-994)payload(996-1058)payload(1060-1068)payload(1070-1140)payload(1352-1417)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
| export type StartAttemptFnParams = Prettify<{ | ||
| ctx: Context; | ||
| init?: InitOutput; | ||
| /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */ | ||
| signal: AbortSignal; | ||
| }>; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Remove init from StartAttempt params (not passed at runtime).
#callOnStartAttemptFunctions() never supplies init, so exposing it here is misleading and can cause incorrect usage.
Apply:
-export type StartAttemptFnParams = Prettify<{
- ctx: Context;
- init?: InitOutput;
- /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */
- signal: AbortSignal;
-}>;
+export type StartAttemptFnParams = Prettify<{
+ ctx: Context;
+ /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */
+ signal: AbortSignal;
+}>;Run to confirm no code depends on init in StartAttempt:
🏁 Script executed:
#!/bin/bash
rg -nP 'StartAttemptFnParams|onStartAttempt\(' -C3Length of output: 3813
Remove init from StartAttemptFnParams (not passed at runtime).
onStartAttempt hooks are invoked without init; the exported type is misleading and should be removed.
Apply:
-export type StartAttemptFnParams = Prettify<{
- ctx: Context;
- init?: InitOutput;
- /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */
- signal: AbortSignal;
-}>;
+export type StartAttemptFnParams = Prettify<{
+ ctx: Context;
+ /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */
+ signal: AbortSignal;
+}>;Files to update: packages/core/src/v3/types/tasks.ts (around lines ~118–124). Also update any SDK hook typings that re-export/reference this type (e.g., packages/trigger-sdk/src/v3/hooks.ts).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type StartAttemptFnParams = Prettify<{ | |
| ctx: Context; | |
| init?: InitOutput; | |
| /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */ | |
| signal: AbortSignal; | |
| }>; | |
| export type StartAttemptFnParams = Prettify<{ | |
| ctx: Context; | |
| /** Abort signal that is aborted when a task run exceeds it's maxDuration or if the task run is cancelled. Can be used to automatically cancel downstream requests */ | |
| signal: AbortSignal; | |
| }>; |
🤖 Prompt for AI Agents
In packages/core/src/v3/types/tasks.ts around lines 118–124, the exported
StartAttemptFnParams type incorrectly includes an optional init property which
is not provided at runtime for onStartAttempt hooks; remove the init property
from that type definition and update any downstream typings that re-export or
reference this type (for example packages/trigger-sdk/src/v3/hooks.ts) to use
the updated StartAttemptFnParams signature without init so the hook type
accurately reflects runtime parameters.
Prevent uncaught errors in the
onSuccess,onComplete, andonFailurelifecycle hooks from failing attempts/runs.Deprecated the
onStartlifecycle hook (which only fires before therunfunction on the first attempt). Replaced withonStartAttemptthat fires before the run function on every attempt:If you want to execute code before just the first attempt, you can use the
onStartAttemptfunction and checkctx.run.attempt.number === 1: